Repository navigation
Fix #218: properly parse stringified JSON array purls in RepositoryId - #244
dhruvv16-hash wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The new test module violates existing test-file header/lint conventions and the updated parsing path can introduce an unhandled json.JSONDecodeError crash for malformed bracketed strings.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR fixes issue #218 by ensuring MapBom.get_purl_from_match() always normalizes RepositoryId (and external IDs) through PurlUtils.parse_purls_from_external_id, so stringified JSON arrays of purls are decoded and the primary purl is selected correctly during BOM mapping.
Changes:
- Update
get_purl_from_match()to routeRepositoryIdthroughPurlUtils.parse_purls_from_external_id()and return the first parsed purl (or""). - Add unit tests to cover both a stringified JSON array
RepositoryIdand a single purlRepositoryId.
File summaries
| File | Description |
|---|---|
capycli/bom/map_bom.py |
Adjusts purl extraction logic so stringified JSON arrays in RepositoryId are parsed before use. |
tests/test_get_purl_from_match.py |
Adds regression tests for RepositoryId purl parsing behavior. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| purls = PurlUtils.parse_purls_from_external_id(raw_purl) | ||
| if purls: | ||
| purl = purls[0] | ||
| return purls[0] | ||
|
|
||
| return purl | ||
| return "" |
| import pytest | ||
| from capycli.bom.map_bom import MapBom | ||
|
|
||
| def test_get_purl_from_match_stringified_array(): | ||
| mb = MapBom() | ||
| # Mocking match with a JSON array string | ||
| match = {"RepositoryId": '["pkg:cargo/clap_builder@4.5.60","pkg:cargo/clap@4.5.60"]'} | ||
| purl = mb.get_purl_from_match(match) | ||
| assert purl == "pkg:cargo/clap_builder@4.5.60" | ||
|
|
||
| def test_get_purl_from_match_single_purl(): | ||
| mb = MapBom() | ||
| match = {"RepositoryId": "pkg:cargo/clap@4.5.60"} | ||
| purl = mb.get_purl_from_match(match) | ||
| assert purl == "pkg:cargo/clap@4.5.60" |
|
|
Hi @dhruvv16-hash! Perhaps you can explain a bit better what the objective of your change is. It definitely doesn't fix #218, at least not the aspects discussed at the end, see especially #218 (comment). Instead, what you seem to do here is to extend #219 to BOMs which use "RepositoryId" instead of "ExternalId" and I wonder where this would come from in your case. Unfortunately, your test cases just do a unit test of the exact changes you applied to the helper, but don't show the real user-facing problem you want to address here. |
|
Hi @gernot-h! My apologies, I was working off an older understanding of the issue and completely missed the consensus at the end of #218 to drop the purl and warn instead of picking the first one (as well as missing your PR #235 entirely!). I'll close this PR to avoid further confusion. Thanks for the heads up! |
Fixes #218
Description
This PR addresses issue #218 by correctly parsing stringified JSON arrays of purls in
RepositoryIdduring BOM mapping.Previously,
get_purl_from_matchwas directly returningmatch["RepositoryId"]without passing it throughPurlUtils.parse_purls_from_external_id. This caused an issue when SW360 mapping resulted in a single string that represented a JSON array of multiple purls (e.g.'["pkg:cargo/clap_builder@4.5.60","pkg:cargo/clap@4.5.60"]'), leading to a ValueError downstream inPackageURL.from_string.By falling through to
parse_purls_from_external_id, the stringified JSON array is safely decoded and the primary purl is extracted correctly.Includes tests!